Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) #149

cygnusv · 2023-07-28T15:21:38Z

Type of PR:

Required reviews:

1
2
3

What this does:
Some of the changes introduced by this PR, all of them related with how ciphertexts are produced and used:

Actually use the AAD input of the AEAD API of Chacha20Poly1305
- Closes Use AAD as input of Chacha20Poly1305.encrypt() #146
Decouple the bulk of symmetric ciphertext from validation; instead just use the hash. This allows to have a small (and also constant!) decryption request.
- Closes Instead of the symmetric ciphertext, pass its hash when creating/checking the auth_tag #147
- Implies changes in nucypher-core and nucypher (TBD)
Refactor Ciphertext implementation.
- Fixes Remove unused construct_tag_hash and Ciphertext.check() #144
- Remove unused & incorrect ciphertext length method (see Serialize Ciphertext G1 and G2 points in compressed form #145 )

Apart from this, bumps MSRV to 1.67.0, and associated clippy changes.

Why it's needed:
@jMyles and @KPrasch identified an important limitation of current ciphertexts and TDec requests construction. Essentially, the TDec request included the full ciphertext, which implies a big overhead even for relatively small media files (e.g. short audio clips). The main goal of this PR was to decouple the bulk of symmetric ciphertext from the validation procedure, using a hash instead. This enables very small TDec requests ( see #147). In the nucypher side, will allow to close nucypher/nucypher#3176 via nucypher/nucypher#3194

Notes for reviewers:

What should reviewers focus on?
Is there a particular commit/function/section of your PR that requires more attention from reviewers?

codecov-commenter · 2023-07-31T09:28:08Z

Codecov Report

Merging #149 (4337c3c) into main (03b4e35) will increase coverage by 0.43%.
The diff coverage is 86.44%.

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   77.96%   78.40%   +0.43%     
==========================================
  Files          23       23              
  Lines        4979     4969      -10     
==========================================
+ Hits         3882     3896      +14     
+ Misses       1097     1073      -24

Files Changed	Coverage Δ
ferveo-common/src/lib.rs	`0.00% <0.00%> (ø)`
ferveo/src/bindings_python.rs	`52.22% <0.00%> (+0.23%)`	⬆️
ferveo/src/bindings_wasm.rs	`0.00% <0.00%> (ø)`
tpke/src/ciphertext.rs	`100.00% <100.00%> (+10.58%)`	⬆️
tpke/src/context.rs	`98.03% <100.00%> (-0.15%)`	⬇️
tpke/src/decryption.rs	`69.92% <100.00%> (ø)`

piotr-roslaniec · 2023-07-31T19:46:14Z

The error on CI is unrelated to this PR. In order to fix it, bump MSRV to 1.67.
Edit: Removed previous workaround.

piotr-roslaniec

LGTM 🎉

(I realize this is WIP, please feel free to re-request a review anytime)

* Remove unused Ciphertext.construct_tag_hash() * Refactor Ciphertext.check() to take the functionality of check_ciphertext_validity() function

Closes #147

See https://docs.rs/aead/0.5.1/aead/struct.Payload.html

derekpierre · 2023-08-03T20:10:15Z

@cygnusv, so Ciphertext comprises of:

    #[serde_as(as = "serialization::SerdeAs")]
    pub commitment: E::G1Affine,
    // W
    #[serde_as(as = "serialization::SerdeAs")]
    pub auth_tag: E::G2Affine,
    // V
    #[serde(with = "serde_bytes")]
    pub ciphertext: Vec<u8>,

So from a dem_ciphertext and kem_ciphertext perspective, what is the kem_ciphertext here? i.e. what's the ciphertext data I would put in the ThresholdDecryptionRequest to send to Ursulas?

cygnusv · 2023-08-03T20:28:46Z

@cygnusv, so Ciphertext comprises of:
    #[serde_as(as = "serialization::SerdeAs")]
    pub commitment: E::G1Affine,
    // W
    #[serde_as(as = "serialization::SerdeAs")]
    pub auth_tag: E::G2Affine,
    // V
    #[serde(with = "serde_bytes")]
    pub ciphertext: Vec<u8>,
So from a dem_ciphertext and kem_ciphertext perspective, what is the kem_ciphertext here? i.e. what's the ciphertext data I would put in the ThresholdDecryptionRequest to send to Ursulas?

Yeah, good question. For the request you only need the commitment, the auth_tag, and the hash of the symmetric ciphertext.

BTW, I'm not sure the term Kem/Dem is technically correct once we introduce the hash of the Dem, but I don't have a better suggestion for the moment.

derekpierre · 2023-08-10T18:30:13Z

Yeah, good question. For the request you only need the commitment, the auth_tag, and the hash of the symmetric ciphertext.

BTW, I'm not sure the term Kem/Dem is technically correct once we introduce the hash of the Dem, but I don't have a better suggestion for the moment.

@cygnusv, 👍 thanks for the clarification. Could we do a quick chat about this whenever you are around?

derekpierre

🎸

derekpierre · 2023-08-11T15:46:58Z

@cygnusv, 👍 thanks for the clarification. Could we do a quick chat about this whenever you are around?

Chatted with @cygnusv , and there will be a follow-up PR to hide some of the Ferveo-specific math details behind higher-level objects.

cygnusv · 2023-08-11T17:14:13Z

@cygnusv, 👍 thanks for the clarification. Could we do a quick chat about this whenever you are around?

Chatted with @cygnusv , and there will be a follow-up PR to hide some of the Ferveo-specific math details behind higher-level objects.

Added new issue #154 to track this

tpke/src/ciphertext.rs

jMyles · 2023-08-13T08:41:41Z

@jMyles and @KPrasch identified an important limitation of current ciphertexts and TDec requests construction.

Credit to Fiddle Kuba ( @kubic71 ) as well.

cygnusv force-pushed the thin branch 2 times, most recently from 7a90b1a to 1c814b8 Compare July 31, 2023 09:10

cygnusv requested review from piotr-roslaniec, jMyles, KPrasch and derekpierre July 31, 2023 14:55

cygnusv changed the title ~~[WIP] Thin Ciphertexts~~ [WIP] Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) Jul 31, 2023

cygnusv added the ⚠️ breaking change label Jul 31, 2023

piotr-roslaniec approved these changes Aug 1, 2023

View reviewed changes

cygnusv added 3 commits August 1, 2023 11:49

Refactor Ciphertext implementation. Fixes #144

41e5be5

* Remove unused Ciphertext.construct_tag_hash() * Refactor Ciphertext.check() to take the functionality of check_ciphertext_validity() function

Use symmetric ciphertext hash when creating/checking the auth_tag

396b1d2

Closes #147

Use AEAD payloads (message + AAD) as input to chacha20poly1305. Fix #146

e662e17

See https://docs.rs/aead/0.5.1/aead/struct.Payload.html

cygnusv force-pushed the thin branch from cc830bf to 46f6fe6 Compare August 1, 2023 09:50

cygnusv marked this pull request as ready for review August 1, 2023 10:04

cygnusv changed the title ~~[WIP] Revamping Ferveo Ciphertexts (breaking changes ⚠️ )~~ Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) Aug 1, 2023

cygnusv mentioned this pull request Aug 1, 2023

Serialize Ciphertext G1 and G2 points in compressed form #145

Closed

cygnusv added 2 commits August 1, 2023 13:39

Test for bad AAD input

e51e6ec

Bump MSRV to 1.67.0

0ec84b6

cygnusv force-pushed the thin branch from 46f6fe6 to da7eb57 Compare August 1, 2023 11:43

cygnusv added 2 commits August 1, 2023 14:05

Remove unused & incorrect ciphertext length method

f427f0d

Clippy stuff

4337c3c

cygnusv force-pushed the thin branch from da7eb57 to 4337c3c Compare August 2, 2023 13:52

cygnusv mentioned this pull request Aug 8, 2023

Add TMK file format support nucypher/taco-web#254

Closed

derekpierre approved these changes Aug 11, 2023

View reviewed changes

derekpierre mentioned this pull request Aug 11, 2023

AccessControlPolicy (ACP) and ThresholdMessageKit nucypher/nucypher#3194

Merged

7 tasks

cygnusv mentioned this pull request Aug 11, 2023

Add header and payload to Ciphertext bindings #154

Closed

jMyles reviewed Aug 11, 2023

View reviewed changes

tpke/src/ciphertext.rs Show resolved Hide resolved

derekpierre mentioned this pull request Aug 16, 2023

Changes needed for ACP, ThresholdMessageKit work in nucypher-core #156

Merged

8 tasks

piotr-roslaniec mentioned this pull request Aug 17, 2023

Update Ciphertext bindings #155

Merged

derekpierre mentioned this pull request Aug 21, 2023

Max size for decryption request ciphertext nucypher/nucypher#3176

Closed

KPrasch approved these changes Aug 22, 2023

View reviewed changes

KPrasch merged commit f44e1be into nucypher:main Aug 22, 2023
9 checks passed

derekpierre mentioned this pull request Aug 30, 2023

v7.0.0 Major feature freeze nucypher/nucypher#3200

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) #149

Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) #149

cygnusv commented Jul 28, 2023 •

edited

Loading

codecov-commenter commented Jul 31, 2023 •

edited

Loading

piotr-roslaniec commented Jul 31, 2023 •

edited

Loading

piotr-roslaniec left a comment •

edited

Loading

derekpierre commented Aug 3, 2023

cygnusv commented Aug 3, 2023

derekpierre commented Aug 10, 2023

derekpierre left a comment

derekpierre commented Aug 11, 2023

cygnusv commented Aug 11, 2023

jMyles commented Aug 13, 2023

Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) #149

Revamping Ferveo Ciphertexts (breaking changes ⚠️ ) #149

Conversation

cygnusv commented Jul 28, 2023 • edited Loading

codecov-commenter commented Jul 31, 2023 • edited Loading

Codecov Report

piotr-roslaniec commented Jul 31, 2023 • edited Loading

piotr-roslaniec left a comment • edited Loading

Choose a reason for hiding this comment

derekpierre commented Aug 3, 2023

cygnusv commented Aug 3, 2023

derekpierre commented Aug 10, 2023

derekpierre left a comment

Choose a reason for hiding this comment

derekpierre commented Aug 11, 2023

cygnusv commented Aug 11, 2023

jMyles commented Aug 13, 2023

cygnusv commented Jul 28, 2023 •

edited

Loading

codecov-commenter commented Jul 31, 2023 •

edited

Loading

piotr-roslaniec commented Jul 31, 2023 •

edited

Loading

piotr-roslaniec left a comment •

edited

Loading